Change the desugaring of `assert!` for better error output #122661

estebank · 2024-03-17T21:19:33Z

In the desugaring of assert!, we now expand to a match expression instead of if !cond {..}.

The span of incorrect conditions will point only at the expression, and not the whole assert! invocation.

error[E0308]: mismatched types
  --> $DIR/issue-14091.rs:2:13
   |
LL |     assert!(1,1);
   |             ^ expected `bool`, found integer

We no longer mention the expression needing to implement the Not trait.

error[E0308]: mismatched types
  --> $DIR/issue-14091-2.rs:15:13
   |
LL |     assert!(x, x);
   |             ^ expected `bool`, found `BytePos`

Now assert!(val) desugars to:

match val {
    true => {},
    _ => $crate::panic::panic_2021!(),
}

Fix #122159.

rustbot · 2024-03-17T21:19:41Z

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-03-17T21:19:43Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

The Miri subtree was changed

cc @rust-lang/miri

src/tools/miri/tests/pass/binops.rs

tests/ui/issues/issue-14091-2.stderr

estebank · 2024-03-17T22:41:11Z

Sigh, clippy shows at least one test where a suggestion causes there to be a condition that isn't a bool but rather a type where Not returns a bool. We can easily continue supporting that by making the desugaring !!cond, but would rather not do that. This happens when it suggest changing assert_eq!(non_bool, true), ending up as assert!(non_bool), which doesn't work with this change .

pnkfelix · 2024-03-18T15:18:59Z

Isn't this technically a breaking change for e.g. (playground):

struct Booly(i32);

impl std::ops::Not for Booly {
    type Output = bool;
    fn not(self) -> Self::Output {
        self.0 == 0
    }
}

fn main() {
    assert!(Booly(1), "booly booly!")
}

pnkfelix · 2024-03-18T15:25:11Z

At the very least, we might need to tie such a change to an edition.

I am not certain whether this decision would be a T-lang matter or a T-libs-api one. I'll nominate for T-lang for now.

(Namely: The question is whether we can start enforcing a rule that the first expression to assert! must be of bool type, which is how the macro is documented, but its current behavior is a little bit more general, as demonstrated in my prior comment)

@rustbot label +I-lang-nominated

src/tools/rust-analyzer/crates/parser/test_data/parser/ok/0035_weird_exprs.rs

estebank · 2024-03-18T15:33:43Z

@pnkfelix we can keep the current (undocumented) behavior by making the desugaring be

{
    let x: bool = !!condition;
    x
}

instead of what this PR does:

{
    let x: bool = condition;
    x
}

I believe that would still cause errors to complain about Not not being implemented, instead of the more straightforward type error, albeit with a better span. I don't particularly like the idea of keeping the current emergent behavior if there aren't people exploiting it in crates.io.

Edit: an option would be to have an internal marker trait:

use std::ops::Not;
trait CanAssert {}
impl<T: Not<Output = bool>> CanAssert for T {}

fn main() {
    let _ = Box::new(true) as Box<dyn CanAssert>;
    let _ = Box::new(42) as Box<dyn CanAssert>;
}

error[E0271]: type mismatch resolving `<i32 as Not>::Output == bool`
 --> src/main.rs:7:13
  |
7 |     let _ = Box::new(42) as Box<dyn CanAssert>;
  |             ^^^^^^^^^^^^ expected `bool`, found `i32`
  |
note: required for `i32` to implement `CanAssert`
 --> src/main.rs:3:29
  |
3 | impl<T: Not<Output = bool>> CanAssert for T {}
  |             -------------   ^^^^^^^^^     ^
  |             |
  |             unsatisfied trait bound introduced here
  = note: required for the cast from `Box<i32>` to `Box<dyn CanAssert>`

pnkfelix · 2024-03-18T17:00:27Z

@estebank what about making the expansion edition-dependent? Is there precedent for that?

Then, editions >= 2024 would expand to what you have proposed in the code of this PR, and editions < 2024 could expand to the !!condition variant form that you have discussed in the comments?

pnkfelix · 2024-03-18T17:53:23Z

what about making the expansion edition-dependent? Is there precedent for that?

(to answer my own question, panic! is one obvious precedent here. So it seems like making it edition-dependent would be one acceptable path; no opinion yet as to which is best...)

rustbot · 2024-03-19T00:05:12Z

Some changes occurred in coverage tests.

cc @Zalathar

estebank · 2024-03-19T00:05:14Z

I tried the marker trait approach for <=2021, and it kind of worked, but the diagnostics were actually worse than just doing { let x: bool = !!$expr; x }, which accounts for pretty much everything we currently support, but with better spans and better errors (if typeof($expr) implements <Not<Output = NotBool>, we now produce an appropriate E0308 type error).

compiler-errors · 2024-03-19T16:40:43Z

Since I don't think it's been acknowledged above, for the record, this breaks the following code:

fn hello(x: &bool) {
  assert!(x);
}

Because &bool: Not<Output = bool>.

estebank · 2024-03-19T17:27:56Z

@compiler-errors that is indeed the case for 2024 onwards, not for previous editions.

pnkfelix · 2024-03-20T14:45:31Z

@compiler-errors that is indeed the case for 2024 onwards, not for previous editions.

I think the critical point is whether an edition-dependent expansion is worth breaking that case (of assert!(x) where x: &bool), or if we should do a non-breaking non-edition-dependent expansion using the let x: bool = !!$expr trick across the board...

Update: I don't know whether it is worth going through this exercise explicitly, but there is a design space here. E.g. one set of options is:

(stable Rust behavior): in all editions, support arbitrary impl Not<Output=bool> for first parameter to assert!;
in edition >= 2024, support just Deref<Target=bool> for first parameter to assert! (e.g. by expanding to let x: &bool = &$expr;), or
(this PR): in edition >= 2024, support just bool for first parameter to assert!.

(And then there's variations thereof about how to handle editions < 2024, but that's a separate debate IMO.)

pnkfelix · 2024-03-22T16:12:22Z

(this is waiting for a decision from T-lang and/or T-libs regarding what interface we want to commit to for assert!)

@rustbot label: +S-waiting-on-team -S-waiting-on-review

estebank · 2025-08-14T18:46:10Z

@bors r=petrochenkov

bors · 2025-08-14T18:46:13Z

📌 Commit 9dfee2e has been approved by petrochenkov

It is now in the queue for this repository.

…ochenkov Change the desugaring of `assert!` for better error output In the desugaring of `assert!`, we now expand to a `match` expression instead of `if !cond {..}`. The span of incorrect conditions will point only at the expression, and not the whole `assert!` invocation. ``` error[E0308]: mismatched types --> $DIR/issue-14091.rs:2:13 | LL | assert!(1,1); | ^ expected `bool`, found integer ``` We no longer mention the expression needing to implement the `Not` trait. ``` error[E0308]: mismatched types --> $DIR/issue-14091-2.rs:15:13 | LL | assert!(x, x); | ^ expected `bool`, found `BytePos` ``` Now `assert!(val)` desugars to: ```rust match val { true => {}, _ => $crate::panic::panic_2021!(), } ``` Fix rust-lang#122159.

Rollup of 22 pull requests Successful merges: - #118087 (Add Ref/RefMut try_map method) - #122661 (Change the desugaring of `assert!` for better error output) - #140740 (Add `-Zindirect-branch-cs-prefix`) - #142640 (Implement autodiff using intrinsics) - #143075 (compiler: Allow `extern "interrupt" fn() -> !`) - #144865 (Fix tail calls to `#[track_caller]` functions) - #144944 (E0793: Clarify that it applies to unions as well) - #144947 (Fix description of unsigned `checked_exact_div`) - #145004 (Couple of minor cleanups) - #145005 (strip prefix of temporary file names when it exceeds filesystem name length limit) - #145012 (Tail call diagnostics to include lifetime info) - #145065 (resolve: Introduce `RibKind::Block`) - #145120 (llvm: Accept new LLVM lifetime format) - #145189 (Weekly `cargo update`) - #145235 (Minor `[const]` tweaks) - #145275 (fix(compiler/rustc_codegen_llvm): apply `target-cpu` attribute) - #145322 (Resolve the prelude import in `build_reduced_graph`) - #145331 (Make std use the edition 2024 prelude) - #145369 (Do not ICE on private type in field of unresolved struct) - #145378 (Add `FnContext` in parser for diagnostic) - #145389 ([rustdoc] Revert "rustdoc search: prefer stable items in search results") - #145392 (coverage: Remove intermediate data structures from mapping creation) r? `@ghost` `@rustbot` modify labels: rollup

…ochenkov Change the desugaring of `assert!` for better error output In the desugaring of `assert!`, we now expand to a `match` expression instead of `if !cond {..}`. The span of incorrect conditions will point only at the expression, and not the whole `assert!` invocation. ``` error[E0308]: mismatched types --> $DIR/issue-14091.rs:2:13 | LL | assert!(1,1); | ^ expected `bool`, found integer ``` We no longer mention the expression needing to implement the `Not` trait. ``` error[E0308]: mismatched types --> $DIR/issue-14091-2.rs:15:13 | LL | assert!(x, x); | ^ expected `bool`, found `BytePos` ``` Now `assert!(val)` desugars to: ```rust match val { true => {}, _ => $crate::panic::panic_2021!(), } ``` Fix rust-lang#122159.

Rollup of 21 pull requests Successful merges: - #118087 (Add Ref/RefMut try_map method) - #122661 (Change the desugaring of `assert!` for better error output) - #142640 (Implement autodiff using intrinsics) - #143075 (compiler: Allow `extern "interrupt" fn() -> !`) - #144865 (Fix tail calls to `#[track_caller]` functions) - #144944 (E0793: Clarify that it applies to unions as well) - #144947 (Fix description of unsigned `checked_exact_div`) - #145004 (Couple of minor cleanups) - #145005 (strip prefix of temporary file names when it exceeds filesystem name length limit) - #145012 (Tail call diagnostics to include lifetime info) - #145065 (resolve: Introduce `RibKind::Block`) - #145120 (llvm: Accept new LLVM lifetime format) - #145189 (Weekly `cargo update`) - #145235 (Minor `[const]` tweaks) - #145275 (fix(compiler/rustc_codegen_llvm): apply `target-cpu` attribute) - #145322 (Resolve the prelude import in `build_reduced_graph`) - #145331 (Make std use the edition 2024 prelude) - #145369 (Do not ICE on private type in field of unresolved struct) - #145378 (Add `FnContext` in parser for diagnostic) - #145389 ([rustdoc] Revert "rustdoc search: prefer stable items in search results") - #145392 (coverage: Remove intermediate data structures from mapping creation) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #122661 - estebank:assert-macro-span, r=petrochenkov Change the desugaring of `assert!` for better error output In the desugaring of `assert!`, we now expand to a `match` expression instead of `if !cond {..}`. The span of incorrect conditions will point only at the expression, and not the whole `assert!` invocation. ``` error[E0308]: mismatched types --> $DIR/issue-14091.rs:2:13 | LL | assert!(1,1); | ^ expected `bool`, found integer ``` We no longer mention the expression needing to implement the `Not` trait. ``` error[E0308]: mismatched types --> $DIR/issue-14091-2.rs:15:13 | LL | assert!(x, x); | ^ expected `bool`, found `BytePos` ``` Now `assert!(val)` desugars to: ```rust match val { true => {}, _ => $crate::panic::panic_2021!(), } ``` Fix #122159.

estebank · 2025-08-15T16:05:46Z

Thank you to everyone who spent time reviewing this PR!

Account for new `assert!` desugaring in `!condition` suggestion `rustc` in rust-lang/rust#122661 is going to change the desugaring of `assert!` to be ```rust match condition { true => {} _ => panic!(), } ``` which will make the edge-case of `condition` being `impl Not<Output = bool>` while not being `bool` itself no longer a straightforward suggestion, but `!!condition` will coerce the expression to be `bool`, so it can be machine applicable. Transposing rust-lang/rust-clippy#15453 to the rustc repo. r? `````@samueltardieu`````

Rollup of 21 pull requests Successful merges: - rust-lang/rust#118087 (Add Ref/RefMut try_map method) - rust-lang/rust#122661 (Change the desugaring of `assert!` for better error output) - rust-lang/rust#142640 (Implement autodiff using intrinsics) - rust-lang/rust#143075 (compiler: Allow `extern "interrupt" fn() -> !`) - rust-lang/rust#144865 (Fix tail calls to `#[track_caller]` functions) - rust-lang/rust#144944 (E0793: Clarify that it applies to unions as well) - rust-lang/rust#144947 (Fix description of unsigned `checked_exact_div`) - rust-lang/rust#145004 (Couple of minor cleanups) - rust-lang/rust#145005 (strip prefix of temporary file names when it exceeds filesystem name length limit) - rust-lang/rust#145012 (Tail call diagnostics to include lifetime info) - rust-lang/rust#145065 (resolve: Introduce `RibKind::Block`) - rust-lang/rust#145120 (llvm: Accept new LLVM lifetime format) - rust-lang/rust#145189 (Weekly `cargo update`) - rust-lang/rust#145235 (Minor `[const]` tweaks) - rust-lang/rust#145275 (fix(compiler/rustc_codegen_llvm): apply `target-cpu` attribute) - rust-lang/rust#145322 (Resolve the prelude import in `build_reduced_graph`) - rust-lang/rust#145331 (Make std use the edition 2024 prelude) - rust-lang/rust#145369 (Do not ICE on private type in field of unresolved struct) - rust-lang/rust#145378 (Add `FnContext` in parser for diagnostic) - rust-lang/rust#145389 ([rustdoc] Revert "rustdoc search: prefer stable items in search results") - rust-lang/rust#145392 (coverage: Remove intermediate data structures from mapping creation) r? `@ghost` `@rustbot` modify labels: rollup

Mark-Simulacrum · 2025-08-19T02:31:30Z

Per #145423 (comment), it looks like this caused a meaningful regression to ripgrep builds... about 0.8% slower:

My guess is that ripgrep contains a particularly large amount of assertions compared to our other primary benchmarks and so it's most affected, but presumably the new expansion is harder on the compiler? It looks like the MIR produced is ~identical between the changed expressions if I correctly skimmed the desugaring godbolt, so it's probably the earlier stages of the compiler that are affected. The match does seem like it consumes more blocks etc.

I'm guessing it's not worth digging too much deeper here, so probably can be marked as triaged. Runtime performance isn't likely to be affected given the equivalent MIR which matters more for this change IMO.

durin42 · 2025-08-20T15:54:48Z

We've discovered that https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=81a747b4884e11637c33beeb1de28465 compiles in stable, but not after this PR. Do you want a tracking bug for that?

samueltardieu · 2025-08-20T16:01:12Z

We've discovered that https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=81a747b4884e11637c33beeb1de28465 compiles in stable, but not after this PR. Do you want a tracking bug for that?

I believe this was discussed in #122661 (comment) and later comments.

estebank · 2025-08-20T18:08:10Z

@durin42 could you point us to where that change in behavior affected you? We tried our best at doing our due diligence to avoid that behavior affecting the ecosystem, but if there was a failure in our methodology it is best to know about it so that we can revise our process in for future changes.

The release notes will highlight this behavioral change.

durin42 · 2025-08-20T18:18:42Z

We saw it in the bitvec crate. ferrilab/bitvec#298 was filed before we realized that backing out this PR let us build bitvec again.

Sounds like we should locally-patch bitvec?

estebank · 2025-08-20T18:24:24Z

I'm shocked this wasn't picked up by our crater runs :-/

We had an earlier proposed approach to restrict the new desugaring to the latest edition, which would have helped/masked this one case. I am concerned that there might have been more cases out there.

saethlin · 2025-08-20T19:34:48Z

I'm shocked this wasn't picked up by our crater runs :-/

bitvec is prepare-fail in at least the second crater run above, along with 135,940 other crates.

https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/crater.3A.20.3E100k.20.22prepare-fail.22.20crates.3F

Skgland · 2025-08-20T21:53:16Z

I'm shocked this wasn't picked up by our crater runs :-/

bitvec is prepare-fail in at least the second crater run above, along with 135,940 other crates.

https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/crater.3A.20.3E100k.20.22prepare-fail.22.20crates.3F

This scenario was basically what my addition of the prepare-fail category was trying to prevent.
By moving spurious failures during crate preparation into their own category and including that category in the retry-regeressed-list.txt they were supposed to be included when a retry is done to prevent the spurious failure from masking an actual regression. Though that obviously only works if the crater run is actually retried with the retry-regressed-list.txt .

Account for new `assert!` desugaring in `!condition` suggestion `rustc` in rust-lang/rust#122661 is going to change the desugaring of `assert!` to be ```rust match condition { true => {} _ => panic!(), } ``` which will make the edge-case of `condition` being `impl Not<Output = bool>` while not being `bool` itself no longer a straightforward suggestion, but `!!condition` will coerce the expression to be `bool`, so it can be machine applicable. Transposing rust-lang#15453 to the rustc repo. r? `````@samueltardieu`````

…ochenkov Change the desugaring of `assert!` for better error output In the desugaring of `assert!`, we now expand to a `match` expression instead of `if !cond {..}`. The span of incorrect conditions will point only at the expression, and not the whole `assert!` invocation. ``` error[E0308]: mismatched types --> $DIR/issue-14091.rs:2:13 | LL | assert!(1,1); | ^ expected `bool`, found integer ``` We no longer mention the expression needing to implement the `Not` trait. ``` error[E0308]: mismatched types --> $DIR/issue-14091-2.rs:15:13 | LL | assert!(x, x); | ^ expected `bool`, found `BytePos` ``` Now `assert!(val)` desugars to: ```rust match val { true => {}, _ => $crate::panic::panic_2021!(), } ``` Fix rust-lang#122159.

rustbot assigned pnkfelix Mar 17, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 17, 2024

estebank commented Mar 17, 2024

View reviewed changes

src/tools/miri/tests/pass/binops.rs Outdated Show resolved Hide resolved

estebank commented Mar 17, 2024

View reviewed changes

tests/ui/issues/issue-14091-2.stderr Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank force-pushed the assert-macro-span branch from ead1593 to eb411c1 Compare March 17, 2024 21:27

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment has been minimized.

Sign in to view

pnkfelix mentioned this pull request Mar 18, 2024

inconsistent and confusing error message about first argument of assert! #122159

Closed

rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Mar 18, 2024

Veykril reviewed Mar 18, 2024

View reviewed changes

src/tools/rust-analyzer/crates/parser/test_data/parser/ok/0035_weird_exprs.rs Outdated Show resolved Hide resolved

estebank force-pushed the assert-macro-span branch from c8185ea to 07a5b21 Compare March 18, 2024 23:59

This comment has been minimized.

Sign in to view

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 14, 2025

Zalathar mentioned this pull request Aug 15, 2025

Rollup of 22 pull requests #145418

Closed

Zalathar mentioned this pull request Aug 15, 2025

Rollup of 21 pull requests #145423

Merged

bors merged commit 1eeb8e8 into rust-lang:master Aug 15, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 15, 2025

github-actions bot mentioned this pull request Aug 18, 2025

Toolchain upgrade to nightly-2025-08-18 flux-rs/flux#1249

Merged

Mark-Simulacrum added perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. labels Aug 19, 2025

cramertj mentioned this pull request Aug 22, 2025

assert desugaring change is backwards-incompatible #145770

Open

Change the desugaring of assert! for better error output #122661

Change the desugaring of assert! for better error output #122661

Conversation

estebank commented Mar 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 17, 2024

Uh oh!

rustbot commented Mar 17, 2024

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

estebank commented Mar 17, 2024

Uh oh!

This comment was marked as resolved.

This comment has been minimized.

pnkfelix commented Mar 18, 2024

Uh oh!

pnkfelix commented Mar 18, 2024

Uh oh!

Uh oh!

estebank commented Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pnkfelix commented Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pnkfelix commented Mar 18, 2024

Uh oh!

rustbot commented Mar 19, 2024

Uh oh!

estebank commented Mar 19, 2024

Uh oh!

This comment has been minimized.

compiler-errors commented Mar 19, 2024

Uh oh!

estebank commented Mar 19, 2024

Uh oh!

pnkfelix commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pnkfelix commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estebank commented Aug 14, 2025

Uh oh!

bors commented Aug 14, 2025

Uh oh!

Uh oh!

estebank commented Aug 15, 2025

Uh oh!

Mark-Simulacrum commented Aug 19, 2025

Uh oh!

durin42 commented Aug 20, 2025

Uh oh!

samueltardieu commented Aug 20, 2025

Uh oh!

estebank commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

durin42 commented Aug 20, 2025

Uh oh!

estebank commented Aug 20, 2025

Uh oh!

saethlin commented Aug 20, 2025

Uh oh!

Skgland commented Aug 20, 2025

Uh oh!

Uh oh!

Change the desugaring of `assert!` for better error output #122661

Change the desugaring of `assert!` for better error output #122661

estebank commented Mar 17, 2024 •

edited

Loading

estebank commented Mar 18, 2024 •

edited

Loading

pnkfelix commented Mar 18, 2024 •

edited

Loading

pnkfelix commented Mar 20, 2024 •

edited

Loading

pnkfelix commented Mar 22, 2024 •

edited

Loading

estebank commented Aug 20, 2025 •

edited

Loading